Postgres: Move io to background task. #3891
Conversation
|
I'll be honest, seeing this posted took the wind out of my sails a bit because I was really looking forward to experimenting with this, but there's so much other work that needs to be done anyway that I'm not going to get too hung up on it. I appreciate the initiative. However, I don't have the time or energy to review this properly while trying to get 0.9.0 out the door so I think ideally we could land this in 0.9.1 or 0.9.2 since it should be backwards-compatible anyway. |
|
Right, I learned a lot while working on this. I mean there are many ways to solve this problem, if you'd like you can still experiment and see what solution you like best. I can imagine that this takes some effort to review and don't mind when this is released, I'll rebase when needed. |
This PR moves all the io in the postgres driver to a background task. This should fix some long standing issues with cancellation safety and unblock/allow features like query pipelining, CopyBoth mode (#2924) and the ability to cancel queries. This also simplifies the dropping of
PgTransaction,PgCopyInandPgListener. There is still some work to do to increase performance but that's out of the scope of this PR.Notable changes:
SocketExt: An extension trait forSocketwhich has async methods for reading, writing, flushing and closing.BufferedSocketnow has poll methods for reading messages.BufferedSockethas aSink<&[u8]>impl to send bytes.Pipeis added; a temporary stream of responses from the background worker.Workeris added; the background worker that handles all the io.ReadyForQuerymessages because thewait_until_readymechanism is removed.Sharedis added; a shared structure between the connection and background worker.This is a pretty big change that touches a lot in the postgres driver. I've added a lot of comments to explain my thinking. I'd recommend going commit by commit for reviewing. If there is anything else I can do to make this easier lmk.
Is this a breaking change?
No, this PR only has breaking changes in sqlx-core which is semver exempt. One unit test had to be changed for
PgListenerbecause the buffering mechanism is changed. I don't think that makes this a breaking change and I don't think Hyrum's law applies here.